Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KEP-3998: Job success/completion policy #4062

Merged
merged 11 commits into from
Feb 8, 2024

Conversation

tenzen-y
Copy link
Member

@tenzen-y tenzen-y commented Jun 6, 2023

  • One-line PR description: KEP for an API to define when a Job can be declared as succeeded.
  • Other comments:

@k8s-ci-robot k8s-ci-robot added the kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory label Jun 6, 2023
@k8s-ci-robot k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 6, 2023
@tenzen-y tenzen-y force-pushed the kep-3998 branch 2 times, most recently from 24a688c to 4fc68f1 Compare June 6, 2023 13:10
@tenzen-y
Copy link
Member Author

tenzen-y commented Jun 6, 2023

cc @alculquicondor @mimowo

@tenzen-y
Copy link
Member Author

tenzen-y commented Jun 8, 2023

@alculquicondor @mimowo I updated the API design. Can you check this?

@tenzen-y
Copy link
Member Author

tenzen-y commented Jun 8, 2023

@soltysh Could you take a look at this proposal?

@tenzen-y
Copy link
Member Author

tenzen-y commented Jun 8, 2023

/wg batch

@k8s-ci-robot k8s-ci-robot added the wg/batch Categorizes an issue or PR as relevant to WG Batch. label Jun 8, 2023
@kannon92
Copy link
Contributor

kannon92 commented Jun 10, 2023

Don’t forget to get the ci checks green. Looks like the linter is saying you are missing some fields

@tenzen-y
Copy link
Member Author

Don’t forget to get the ci checks green. Looks like the linter is saying you are missing some fields

Thanks @kannon92! I will check the CI error.

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 11, 2023
@tenzen-y
Copy link
Member Author

Don’t forget to get the ci checks green. Looks like the linter is saying you are missing some fields

Thanks @kannon92! I will check the CI error.

Done.

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
…CronJob ForbidUntilJobSuccessful

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
@tenzen-y
Copy link
Member Author

tenzen-y commented Feb 7, 2024

@wojtek-t I addressed all comments.
Please take another look. Thank you!

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
Copy link
Member

@alculquicondor alculquicondor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still LGTM

keps/sig-apps/3998-job-success-completion-policy/README.md Outdated Show resolved Hide resolved
However, both Job level and JobSet level successPolicies would be valuable
since there are some cases in which we want to launch a Job by a single podTemplate.

<<[UNRESOLVED @tenzen-y: Do we need to make configurable actions? ]>>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the purpose of this iteration, these are not unresolved questions. We have concluded that they might be re-evaluated in a future version. You can mention this in the alpha/beta criteria, but you shouldn't leave the UNRESOLVED tags.

@wojtek-t
Copy link
Member

wojtek-t commented Feb 8, 2024

Just one more comment from the PRR.

I will approve though to not block it and ad hold to have it addressed before submitting.

/approve PRR
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 8, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: soltysh, tenzen-y, wojtek-t

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 8, 2024
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
@wojtek-t
Copy link
Member

wojtek-t commented Feb 8, 2024

/lgtm
/hold cancel

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Feb 8, 2024
Copy link
Member

@alculquicondor alculquicondor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/hold cancel

@alculquicondor
Copy link
Member

race condition :)

@tenzen-y
Copy link
Member Author

tenzen-y commented Feb 8, 2024

race condition :)

The browser is loading this PR so slower, so race conditions often occur :)

@k8s-ci-robot k8s-ci-robot merged commit 6d05986 into kubernetes:master Feb 8, 2024
4 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.30 milestone Feb 8, 2024
@tenzen-y tenzen-y deleted the kep-3998 branch February 8, 2024 19:58
@tenzen-y
Copy link
Member Author

tenzen-y commented Feb 8, 2024

Thank you to everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/apps Categorizes an issue or PR as relevant to SIG Apps. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. wg/batch Categorizes an issue or PR as relevant to WG Batch.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.